remove node types and bun types from workflows#199
Conversation
…st that this setup actually works
product-security-plaid-production
bot
requested review from
mchain0 and
tomasz-ratajczyk-sc
… into feat/no-node-types
| const resp = httpClient.sendRequest(nodeRuntime, req).result() | ||
| // The mathjs.org API returns the result as a raw string in the body. | ||
| // We need to parse it into a bigint. | ||
| const bodyText = new TextDecoder().decode(resp.body) |
There was a problem hiding this comment.
Let's ensure we don't break existing workflows. TextDecoder still needs to work.
There was a problem hiding this comment.
This is just "extra" cleanup in my example as using text shall be more convenient for the users. TextDecoder does in fact still work and you can still use the previous notation if you prefer.
jeffrifwaldsmartcontract
left a comment
jeffrifwaldsmartcontract
left a comment
There was a problem hiding this comment.
This approach makes sense to me. Do we just continue to add libraries to the blacklist as users complain they don't work and we confirm they shouldn't work?
| "noFallthroughCasesInSwitch": true, | ||
|
|
||
| // Do not auto-include @types/* — mirrors the customer cre cli initialized environment | ||
| "types": [], |
There was a problem hiding this comment.
Do we need to make sure the cre-cli tsconfigs match, or do they pull in this file directly now?
There was a problem hiding this comment.
I will follow up with CLI PR that initialize project with this settings.
I'm aiming to do "level 2" PR which would basically go deeper into AST and do analyze libraries, similar to That being said I don't think we would have to do more patching of libraries. Its mostly about platform APIs that are not supported, so we go into library code, check for these, if there are no usages then library itself should be good to go. We could only think about throwing warnings when we do |
3 participants
Summary
This is often coming back and throwing users off guard. Changes proposed in this PR aims to be solid first step to improve that experience.
Compile-time: Type-level restrictions
We're adding custom types to a whole set of common Node built-in modules (
node:crypto,node:fs,node:http,node:net, etc.). This custom type mark those APIs asneverwhich will result in clear errors when running workflows.Our
cre-sdk-exampleswill now present have nowWe also Removed
@types/nodeandbun-typesand ourcre-sdk-exampleswill now behave like user workflows now, correctly showing what is available.Build-time: Static analysis validator
There's an extra step added during workflow compilation that would check for usage of restricted node modules. It would produce readable errors instead of weird WASM traces.
What is not covered
Current implementation doesn't cover 3rd party libraries. For example if I import
my-custom-hasheslibrary which under the hoods is using unavailable NodeJS APIs we wouldn't be able to catch it as nicely as current direct node imports.That's because it would require
skipLibCheckoption set tofalsein TypeScript which is not a common practice and we could get more reports about unrelated 3rd party libs erroring out. I will keep exploring this - but I think current proposal already uplifts DX experience of the SDK a lot.